Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move from dep to go modules #1303

Merged
merged 17 commits into from
May 9, 2019
Merged

Move from dep to go modules #1303

merged 17 commits into from
May 9, 2019

Conversation

daixiang0
Copy link
Contributor

@daixiang0 daixiang0 commented Mar 27, 2019

  • add go module config
  • update Makefile
  • update README
  • update CI config
  • update test tool

Signed-off-by: Xiang Dai <764524258@qq.com>
Signed-off-by: Xiang Dai <764524258@qq.com>
Signed-off-by: Xiang Dai <764524258@qq.com>
@bboreham
Copy link
Contributor

Same comment as I made on #1232: please note the libraries that are updated in this PR, and any interesting changes in behaviour.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Mar 27, 2019

@bboreham hi, take an example that why go-grpc-middleware is mentioned and grpc-gateway is not:

# GO111MODULE=on go mod why github.com/grpc-ecosystem/grpc-gateway
# github.com/grpc-ecosystem/grpc-gateway
(main module does not need package github.com/grpc-ecosystem/grpc-gateway)
# GO111MODULE=on go mod why github.com/grpc-ecosystem/go-grpc-middleware
# github.com/grpc-ecosystem/go-grpc-middleware
github.com/cortexproject/cortex/pkg/util/grpcclient
github.com/grpc-ecosystem/go-grpc-middleware

use go mod why easily to explain.

Also, grpc-gateway is in go.sum indeed:

# git grep grpc-gateway
go.sum:142:github.com/grpc-ecosystem/grpc-gateway v0.0.0-20190311121628-6523154b0128/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
go.sum:143:github.com/grpc-ecosystem/grpc-gateway v1.5.0/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpgL2+G+NZTnrVHpWWfpdw=
go.sum:144:github.com/grpc-ecosystem/grpc-gateway v1.6.3 h1:oQ+8y59SMDn8Ita1Sh4f94XCUVp8AB84sppXP8Qgiow=
go.sum:145:github.com/grpc-ecosystem/grpc-gateway v1.6.3/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpgL2+G+NZTnrVHpWWfpdw=
......

If we depend on one project A, but A does not have go.mod, the dependence of A would be auto found by go mod tidy refer to doc:

If a dependency of your module does not itself have a go.mod (e.g., because the dependency has not yet opted in to modules itself), or if its go.mod file is missing one or more of its dependencies (e.g., because the module author did not run go mod tidy), then the missing transitive dependencies will be added to your module's requirements, along with an // indirect comment to indicate that the dependency is not from a direct import within your module.

@daixiang0
Copy link
Contributor Author

BTW, for go mod, no need vendor. Keep vendor and dep config to support dep and in future, they would be removed if not support dep.

@bboreham
Copy link
Contributor

Thank you for answering the subordinate question.

For clarity, my main request is for a list of the changes to dependency versions, and for some analysis of what is different. This is to guide what we should watch out for when updating.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Mar 28, 2019

i compare Gopkg.lock and go.mod to get following list:
since different rule between dep and go mod, for old version, if no version in Gopkg.lock, i use revision
@bboreham you can double check it.

|name | old | new | | --- | --- | --- | |cloud.google.com/go|ed41f43dafb848a06a7136373760f3c09326fb44|v0.28.0| |contrib.go.opencensus.io/exporter/ocagent|v0.2.0|-| |github.com/Azure/azure-sdk-for-go|a1a2da0aba294fe51ba47119e652ff2f08a78afb|-| |github.com/Azure/go-autorest|v11.5.1|-| |github.com/Masterminds/squirrel|20f192218cf52a73397fa2df45bdda720f3e47c8|v0.0.0-20161115235646-20f192218cf5| |github.com/NYTimes/gziphandler|v1.1.1|v0.0.0-20190221231647-dd0439581c76| |github.com/alecthomas/units|2efee857e7cfd4f3d0138cc3cbb1b4966962b93a|-| |github.com/armon/go-socks5|e75332964ef517daa070d7c38a9466a0d687e0a5|v0.0.0-20160902184237-e75332964ef5| |github.com/aws/aws-sdk-go|v1.15.90|v0.0.0-20181204211515-ddc06f9fad88| |github.com/beorn7/perks|4c0e84591b9aa9e6dcfdf3e020114cd81f89d5f9|-| |github.com/blang/semver|v3.5.0|v0.0.0-20170130170546-b38d23b8782a| |github.com/bradfitz/gomemcache|1952afaa557dc08e8e0d89eafab110fb501c1a2b|v0.0.0-20170208213004-1952afaa557d| |github.com/cenkalti/backoff|v1.0.0|v0.0.0-20160321131554-32cd0c5b3aef // indirect| |github.com/census-instrumentation/opencensus-proto|v0.1.0|-| |github.com/cespare/xxhash|v1.0.0|-| |github.com/codahale/hdrhistogram|3a0bb77429bd3a61596f5e8a3172445844342120|-| |github.com/davecgh/go-spew|v1.1.0|-| |github.com/dgrijalva/jwt-go|v3.2.0|v0.0.0-20180308231308-06ea1031745c // indirect| |github.com/etcd-io/bbolt|v1.3.1-etcd.8|v0.0.0-20180912205654-7ee3ded59d48| |github.com/fluent/fluent-logger-golang|v1.2.1|v0.0.0-20161018015219-28bdb662295c // indirect| |github.com/fsouza/fake-gcs-server|v1.3.0|v0.0.0-20180925145035-9f9efdc50d6a| |github.com/go-kit/kit|v0.5.0|v0.8.0| |github.com/go-logfmt/logfmt|v0.3.0|-| |github.com/go-stack/stack|v1.6.0|-| |github.com/gocql/gocql|697e7c57f99bb127606c4d4f975c2ff4f34eca1c|v0.0.0-20180113133114-697e7c57f99b| |github.com/gogo/googleapis|v1.1.0|v0.0.0-20180813112041-8558fb44d2f1 // indirect| |github.com/gogo/protobuf|v1.1.1|v1.2.0| |github.com/gogo/status|v1.0.3|v0.0.0-20180521094753-d60b5acac426| |github.com/golang/protobuf|v1.2.0|v1.2.0| |github.com/golang/snappy|553a641470496b2327abcac10b36396bd98e45c9|v0.0.0-20170215233205-553a64147049| |github.com/google/btree|e89373fe6b4a7413d7acd6da1725b83ef713e6e4|-| |github.com/google/gofuzz|44d81051d367757e1c7c6a5a86423ece9afcf63c|v0.0.0-20161122191042-44d81051d367 // indirect| |github.com/googleapis/gax-go|v2.0.3|-| |github.com/googleapis/gnostic|v0.2.0|-| |github.com/gophercloud/gophercloud|fe1ba5ce12dda9d3056d303dfe080d5eccdd1780|v0.0.0-20190307220656-fe1ba5ce12dd // indirect| |github.com/gorilla/context|v1.1|v0.0.0-20160226214623-1ea25387ff6f // indirect| |github.com/gorilla/mux|v1.6.2|v1.6.2| |github.com/gregjones/httpcache|9cad4c3443a7200dd6400aef47183728de563a38|-| |github.com/grpc-ecosystem/go-grpc-middleware|v1.0.0|v0.0.0-20180502091642-c250d6563d4d| |github.com/grpc-ecosystem/grpc-gateway|v1.8.3|-| |github.com/hailocab/go-hostpool|e80d13ce29ede4452c43dea11e79b9bc8a15b478|v0.0.0-20160125115350-e80d13ce29ed // indirect| |github.com/hashicorp/consul|v1.1.0|v0.0.0-20180615161029-bed22a81e9fd| |github.com/hashicorp/go-cleanhttp|3573b8b52aa7b37b9358d966a898feb387f62437|v0.0.0-20170211013415-3573b8b52aa7| |github.com/hashicorp/go-rootcerts|6bb64b370b90e7ef1fa532be9e591a81c3493e00|-| |github.com/hashicorp/golang-lru|0fb14efe8c47ae851c0034ed7a448854d3d34cf3|-| |github.com/hashicorp/serf|v0.8.1|v0.0.0-20170206200529-d6574a5bb122 // indirect| |github.com/jmespath/go-jmespath|0b12d6b5|-| |github.com/jonboulle/clockwork|v0.1.0|v0.0.0-20160615175015-2eee05ed7941| |github.com/json-iterator/go|1.1.5|v0.0.0-20180806060727-1624edc4454b| |github.com/julienschmidt/httprouter|v1.1|-| |github.com/kr/logfmt|b84e30acd515aadc4b783ad4ff83aff3299bdfe0|-| |github.com/lann/builder|f22ce00fd9394014049dad11c244859432bd6820|v0.0.0-20150808151131-f22ce00fd939 // indirect| |github.com/lann/ps|62de8c46ede02a7675c4c79c84883eb164cb71e3|v0.0.0-20150810152359-62de8c46ede0 // indirect| |github.com/lib/pq|2704adc878c21e1329f46f6e56a1c387d788ff94|v1.0.0| |github.com/mattes/migrate|v1.3.1|v0.0.0-20170420185049-7dde43471463| |github.com/mattn/go-colorable|v0.0.7|v0.0.0-20161103160040-d22884950486 // indirect| |github.com/mattn/go-isatty|v0.0.2|v0.0.0-20170322234413-fc9e8d8ef484 // indirect| |github.com/mgutz/ansi|9520e82c474b0a04dd04f8a40959027271bab992|v0.0.0-20170206155736-9520e82c474b| |github.com/miekg/dns|6ebcb714d36901126ee2807031543b38c56de963|-| |github.com/mitchellh/go-homedir|b8bc1bf767474819792c23f32d8286a45736f1c6|-| |github.com/modern-go/concurrent|1.0.3|-| |github.com/modern-go/reflect2|1.0.1|-| |github.com/mwitkow/go-conntrack|cc309e4a22231782e8893f3c35ced0967807a33e|-| |github.com/oklog/oklog|v0.2.2|-| |github.com/oklog/ulid|v0.3.0|-| |github.com/opentracing-contrib/go-grpc|4b5a12d3ff02ba61ae861b7797e17a0c4f0ecea9|v0.0.0-20180928155321-4b5a12d3ff02| |github.com/opentracing-contrib/go-stdlib|1de4cc2120e71f745a5810488bf64b29b6d7d9f6|v0.0.0-20170113013457-1de4cc2120e7| |github.com/opentracing/opentracing-go|v1.0.2|v1.0.1| |github.com/petar/GoLLRB|53be0d36a84c2a886ca057d34b6aa4468df9ccb4|v0.0.0-20130427215148-53be0d36a84c // indirect| |github.com/peterbourgon/diskv|v2.0.1|-| |github.com/philhofer/fwd|98c11a7a6ec829d672b03833c3d69a7fae1ca972|v0.0.0-20160129035939-98c11a7a6ec8 // indirect| |github.com/pkg/errors|v0.8.0|v0.8.0| |github.com/pmezard/go-difflib|v1.0.0|-| |github.com/prometheus/alertmanager|fb713f6d8239b57c646cae30f78e8b4b8861a1aa|v0.0.0-20180112102915-fb713f6d8239| |github.com/prometheus/client_golang|bcbbc08eb2ddff3af83bbf11e7ec13b4fd730b6e|v0.9.1| |github.com/prometheus/client_model|6f3806018612930941127f2a7c6c453ba2c527d2|-| |github.com/prometheus/common|7600349dcfe1abd18d72d3a1770870d9800a7801|v0.0.0-20181119215939-b36ad289a3ea| |github.com/prometheus/procfs|6ac8c5d890d415025dd5aae7595bcb2a6e7e2fad|-| |github.com/prometheus/prometheus|59369491cfdfe8dcb325723d6d28a837887a07b9|v0.0.0-20190312040920-59369491cfdf| |github.com/prometheus/tsdb|v0.6.1|v0.6.1| |github.com/samuel/go-zookeeper|1d7be4effb13d2d908342d349d71a284a7542693|-| |github.com/satori/go.uuid|v1.1.0|-| |github.com/segmentio/fasthash|a72b379d632eab4b49e4f4b2c765cfebf0a74796|v0.0.0-20180216231524-a72b379d632e| |github.com/sercand/kuberesolver|v1.0.0|v0.0.0-20171128105722-aa801ca26294 // indirect| |github.com/sirupsen/logrus|v1.0.6|v0.0.0-20180721070001-3e01752db018 // indirect| |github.com/stretchr/testify|v1.1.4|v1.3.0| |github.com/tinylib/msgp|v1.0|v0.0.0-20161221055906-38a6f61a768d // indirect| |github.com/uber/jaeger-client-go|v2.14.0|v0.0.0-20180430180415-b043381d9447| |github.com/uber/jaeger-lib|v1.5.0|v0.0.0-20180511153330-ed3a127ec5fe // indirect| |github.com/weaveworks/billing-client|be0d55e547b147ea1817f037cab9458bf7fc7850|v0.0.0-20171006123215-be0d55e547b1| |github.com/weaveworks/common|c1808abf9c462ba088ef5c764053a316a58cde24|v0.0.0-20181109173936-c1808abf9c46| |github.com/weaveworks/mesh|5015f896ab62d3e9fe757456c757521ce0c3faff|v0.0.0-20170131170447-5015f896ab62| |github.com/weaveworks/promrus|v1.2.0|v0.0.0-20171010152908-0599d764e054 // indirect| |go.opencensus.io|v0.18.0|-| |golang.org/x/crypto|c7af5bf2638a1164f2eb5467c39c6cffbd13a02e|-| |golang.org/x/net|adae6a3d119ae4890b46832a2e88a95adc62b8e7|v0.0.0-20181220203305-927f97764cc3| |golang.org/x/oauth2|d668ce993890a79bda886613ee587a69dd5da7a6|v0.0.0-20181203162652-d668ce993890 // indirect| |golang.org/x/sync|f52d1811a62927559de87708c8913c1650ce4f26|-| |golang.org/x/sys|ec83556a53fe16b65c452a104ea9d1e86a671852|v0.0.0-20190209173611-3b5209105503| |golang.org/x/text|a9a820217f98f7c8a207ec1e45a874e1fe12c478|-| |golang.org/x/time|8be79e1e0910c292df4e79c241bb7e8f7e725959|v0.0.0-20170424234030-8be79e1e0910| |golang.org/x/tools|4bb9a6d30bdc727aebd0747694f31de632a5282e|v0.0.0-20190114222345-bf090417da8b| |google.golang.org/api|6142e720c068c6cd71f2258e007ff1991572e1d5|v0.0.0-20181203233308-6142e720c068| |google.golang.org/appengine|v1.0.0|-| |google.golang.org/genproto|2b5a72b8730b0b16380010cfe5286c42108d88e7|| |google.golang.org/grpc|v1.16.0|v1.19.0| |gopkg.in/check.v1|-|v1.0.0-20180628173108-788fd7840127 // indirect| |gopkg.in/fsnotify/fsnotify.v1|v1.4.7|-| |gopkg.in/inf.v0|v0.9.0|-| |gopkg.in/yaml.v2|v2.2.1|v2.2.2| |k8s.io/api|05914d821849570fba9eacfb29466f2d8d3cd229|-| |k8s.io/apimachinery|2b1284ed4c93a43499e781493253e2ac5959c4fd|-| |k8s.io/client-go|a47917edff34c2c3f7be36398e3ebad6011ce05c|-| |k8s.io/klog|71442cd4037d612096940ceb0f3fec3f7fff66e0|-| |k8s.io/kube-openapi| - | v0.0.0-20180629012420-d83b052f768a| |labix.org/v2/mgo |-| v0.0.0-20140701140051-000000000287| |launchpad.net/gocheck |-|v0.0.0-20140225173054-000000000087| |sigs.k8s.io/yaml |-| v1.1.0|

@bboreham
Copy link
Contributor

Having a list is a good step, but before I deploy in production I like to know what behaviour has changed.

I.e., scan the release notes of each dependency and flag anything that may be relevant.

You’re right it’s a big list. Maybe we can divide and conquer?

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 1, 2019

Having a list is a good step, but before I deploy in production I like to know what behaviour has changed.

It is hard to cover all cases, while now all tests pass.
Too many projects base on this, from my side, get to know what behaviour has changed is very tough.

You’re right it’s a big list. Maybe we can divide and conquer?

Agree, which part does we begin?

@bboreham
Copy link
Contributor

bboreham commented Apr 1, 2019

Start at the top of the list?
Maybe call out “I will do the first 5” to avoid duplication?

@bboreham
Copy link
Contributor

bboreham commented Apr 1, 2019

I'll start:

First line - cloud.google.com/go was ed41f43dafb8 now v0.28.0 - I can't find where Google publishes release notes for this library. Punt and come back.

Second line - contrib.go.opencensus.io/exporter/ocagent was v0.2.0 now "-". But the files are not deleted from vendor/contrib.go.opencensus.io by this PR, they are updated. For instance https://github.com/cortexproject/cortex/pull/1303/files#diff-1151b378d60e59d0ef038ae0f47bcf9dR4
Is this an error in your method for listing changes?
Same for the next two lines.

github.com/Masterminds/squirrel was 20f192218cf5 now v0.0.0-20161115235646-20f192218cf5 - this is actually the same commit. Could make this clearer in the list.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 3, 2019

Is this an error in your method for listing changes?

refer this comment, i only compare between Gopkg.lock and go.mod, not go.sum or vender, as i said in the beginning of comment.

github.com/Masterminds/squirrel was 20f192218cf5 now v0.0.0-20161115235646-20f192218cf5 - this is actually the same commit. Could make this clearer in the list.

It is difference rule between go dep and go mod, let us update list during check :)

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 3, 2019

@bboreham hi, edit list here is too bad, i create a google docs, you can edit it.

For vendor change, i will remove commit about vendor so that all commits only relate to go mod not go dep. WDYT?

@daixiang0
Copy link
Contributor Author

@bboreham hi, any update? The list i has finished.

@bboreham
Copy link
Contributor

bboreham commented Apr 8, 2019

My objective is to know what changed, before I have to support a change in production.

The size of diff in this PR has come down from +900,000 to +100,000, which is good, but still I do not see a change in your table that matches this change in the code:
https://github.com/cortexproject/cortex/pull/1303/files#diff-d2af11b35109d03a552583a9464bdf56R107

I appreciate you don't know either, I'm not saying this is bad. I am saying I can't approve something I don't understand.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 9, 2019

@bboreham thanks for response, i have remove changesd about vendor.

@daixiang0
Copy link
Contributor Author

I get more knowledge in this process, and i find that my env not clean.
I run in a container to ensure clean env then update mod, the list also has been updated.

Signed-off-by: Xiang Dai <764524258@qq.com>
Signed-off-by: Xiang Dai <764524258@qq.com>
Signed-off-by: Xiang Dai <764524258@qq.com>
This reverts commit 9086dc9.

Signed-off-by: Xiang Dai <764524258@qq.com>
Signed-off-by: Xiang Dai <764524258@qq.com>
Signed-off-by: Xiang Dai <764524258@qq.com>
@gouthamve gouthamve force-pushed the mod branch 3 times, most recently from fb8b0bf to 4b85eda Compare May 6, 2019 13:48
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
README.md Outdated Show resolved Hide resolved
@tomwilkie
Copy link
Contributor

Thanks for the PR @daixiang0! @gouthamve is going to do a build of this and stick it in our dev env to test things out. If all is well, we should be able to get this merged.

Signed-off-by: Xiang Dai <764524258@qq.com>
@tomwilkie
Copy link
Contributor

FYI I tried deleing vendor and running go mod vendor, and there were no git changes. Good work!

@@ -137,8 +137,12 @@ shell:
configs-integration-test:
/bin/bash -c "go test -tags 'netgo integration' -timeout 30s ./pkg/configs/... ./pkg/ruler/..."

dep-check:
dep check
mod-check:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomwilkie We make sure there are no changes in CI too :)

Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try it without changing the tools subtree ?

tools/test Outdated Show resolved Hide resolved
tools/test Outdated Show resolved Hide resolved
@bboreham
Copy link
Contributor

bboreham commented May 7, 2019

Has anyone done a forwards/backwards compatibility check on the new protobuf library?
See also #1370

This reverts commit ac682ba.

Signed-off-by: Xiang Dai <764524258@qq.com>
@tomwilkie
Copy link
Contributor

Has anyone done a forwards/backwards compatibility check on the new protobuf library?

We've done 3 rolling upgrades (two dev envs and one prod env) and didn't hit any problems.

I can't see anything that might indicate a compatibility change - and thats kinda the whole point of protobufs.

@tomwilkie
Copy link
Contributor

LGTM

@tomwilkie tomwilkie merged commit e2154c4 into cortexproject:master May 9, 2019
@bboreham
Copy link
Contributor

bboreham commented May 9, 2019

thats kinda the whole point of protobufs

strangely this did not save us in #938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants